fix: scope Stop hook to active workflow only#139
Conversation
The Stop hook scanned all progress files and blocked Claude from stopping when any stale in_progress workflow existed — even from previous sessions or for different tickets. This caused Claude to hijack sessions and work on tickets the user didn't ask about. Introduce an active-workflow marker (.claude/docs/.active-workflow) that the orchestrator writes when starting or resuming a workflow. The hook now reads only this marker, ignoring all other progress files. Stale markers are auto-cleaned when they point to missing or completed workflows. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> rh-pre-commit.version: 2.3.2 rh-pre-commit.check-secrets: ENABLED
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (2)
WalkthroughAdds a single active-workflow sidecar at Changes
Sequence Diagram(s)sequenceDiagram
participant Orchestrator
participant Marker as ".claude/docs/.active-workflow"
participant StopHook
participant Progress as "progress.json"
Orchestrator->>Marker: write/update marker (progress_file, ticket, workflow_type)
Note over Marker: marker represents current active workflow
StopHook->>Marker: read marker
alt marker missing
StopHook->>StopHook: exit 0 (allow stop)
else marker present
StopHook->>StopHook: parse marker (jq)
alt parse failure
StopHook->>StopHook: print error, exit 2
else parse success
StopHook->>Progress: read referenced progress_file
alt progress file missing or marker stale
StopHook->>Marker: delete marker
StopHook->>StopHook: exit 0
else progress read
alt progress.status == "in_progress"
StopHook->>Progress: evaluate step_order/steps and stop_count
alt incomplete steps remain AND stop_count < threshold
StopHook->>StopHook: emit stderr, exit 2 (block stop)
else
StopHook->>Marker: remove marker
StopHook->>StopHook: exit 0
end
else
StopHook->>Marker: remove marker
StopHook->>StopHook: exit 0
end
end
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 1 inconclusive)
✅ Passed checks (8 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Review rate limit: 9/10 reviews remaining, refill in 6 minutes. Comment |
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> rh-pre-commit.version: 2.3.2 rh-pre-commit.check-secrets: ENABLED
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@plugins/docs-tools/skills/docs-orchestrator/hooks/workflow-completion-check.sh`:
- Around line 31-38: The hook currently suppresses jq errors and treats parse
failures as stale state, then deletes the MARKER; change this so PROGRESS_FILE
and TICKET are parsed with jq but you check jq's exit status and do not remove
MARKER on parse errors. Concretely, in workflow-completion-check.sh capture jq's
exit code when reading PROGRESS_FILE and TICKET (references: PROGRESS_FILE,
TICKET, MARKER), and if jq returns non-zero (parse/read failure) exit non-zero
(fail closed) without running rm -f "$MARKER"; only when jq succeeds and both
variables are empty should you remove the marker and exit 0.
In `@plugins/docs-tools/skills/docs-orchestrator/SKILL.md`:
- Around line 337-345: Update the JSON example so the progress_file uses the
lowercased ticket placeholder to match the directory rule: change the path in
progress_file (and the filename token) to use <ticket-lower> (e.g.
".claude/docs/<ticket-lower>/workflow/<workflow-type>_<ticket-lower>.json");
ensure you update the progress_file value and any occurrence of the ticket
placeholder in that example so it consistently references <ticket-lower>
alongside the existing <workflow-type> token.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 7ebc2c26-ef2b-4675-b68d-03ca2db782a9
📒 Files selected for processing (2)
plugins/docs-tools/skills/docs-orchestrator/SKILL.mdplugins/docs-tools/skills/docs-orchestrator/hooks/workflow-completion-check.sh
… path Stop hook now checks jq exit status when parsing .active-workflow marker. Parse failures block stop (exit 2) instead of silently deleting the marker. Also aligns the SKILL.md progress_file example with the lowercase ticket directory convention. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> rh-pre-commit.version: 2.3.2 rh-pre-commit.check-secrets: ENABLED
Summary
in_progressworkflow existed — even from previous sessions or different tickets. This caused Claude to hijack sessions and work on tickets the user didn't ask about.claude/docs/.active-workflow) that the orchestrator writes when starting/resuming a workflow. The hook now reads only this marker, ignoring all other progress filesTest plan
in_progressworkflows from previous sessions no longer trigger the hook (no.active-workflowmarker → hook exits 0)/docs-orchestrator TICKET-1— verify.active-workflowis created and hook blocks correctly mid-workflow🤖 Generated with Claude Code
Summary by CodeRabbit
Documentation
Refactor
Bug Fixes
Chores